perf(string): fuse accumulator concat chains - #8497
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Landed on I read both rooting paths rather than relying on the suites, since this touches in-place string mutation and raw payload pointers held across an allocation. Both are correct: the runtime roots every piece in a Validation: 9 ratchet gates + Also worth noting your A/B methodology was right — identical |
Summary
s = s + a + b + ...for proven string accumulators in local, captured, and module-global slotsRefs #8410.
Profile
The macOS
sampleutility hung before collecting a sample on this host, including when reproduced againstyes. I used the same symbol-preserving build (PERRY_KEEP_SYMBOLS=1 PERRY_DEBUG_SYMBOLS=1, about 17.7k symbols in the generated fixture) with Instruments Time Profiler over a private 10xiso_missfixture instead.Baseline sampled CPU was 8,422 ms. The largest leaves were
evalNode(38.90%),js_string_concat_chain(14.15%),lookup(12.19%),memmove(4.19%), arena allocation (3.72%), and the separate accumulator append/storage/rooting envelope (6.80% combined).Candidate sampled CPU was 7,716 ms (-8.38%). The suffix concat plus separate append envelope becomes one
js_string_append_chainleaf (15.15%);evalNoderemained dominant (42.69%) andlookupremained essentially unchanged (12.08%).Performance
Five shuffled, interleaved repeats on macOS, comparing a locally built
8d1cca158baseline with this branch. Both arms used the identical-p perry -p perry-runtime-static -p perry-stdlib-staticrelease build set, and archive mtimes were verified after each build. Medians are shown.iso_missThe wall ranges overlap widely (1.41-5.00 s before, 1.24-7.07 s after), so the wall median is not treated as a real 28.9% result. Instructions retired are the primary signal. The observed RSS cost is 48 KiB; the implementation does not add a cache or retained reserve and allocates the complete result once, but the measured peak is reported rather than rounded away.
Whole-corpus medians:
No non-target row moved more than 0.41% in instructions. All non-target wall ranges overlapped; the large wall/cycle swings track host contention rather than instruction count.
Validation
cargo test --release -p perry-runtime --lib: 2,608 passed, 4 ignoredcargo test --release -p perry-codegen --lib: 1,121 passedcargo test --release -p perry --bin perry: 1,008 passedHost status at handoff: 6 GiB free after deleting this worktree's generated
target/and reducing the reserved external target to about 832 MiB. The task requires stopping below 15 GiB, so this PR remains draft until those two acceptance runs can complete.